Fix #11093, #12377 FP mismatchingContainerIterator#5911
Conversation
chrchr-github
changed the title
| ValueFlow::Value val = getLifetimeIteratorValue(iterTok); | ||
| if (!val.tokvalue) | ||
| continue; | ||
| if (!val.isKnown() && Token::simpleMatch(val.tokvalue->astParent(), ":")) |
There was a problem hiding this comment.
The lifetime values are never known. Also, this will lead to FNs as well. I think it would be better to check that the set of lifetimes are the same when there are multiple lifetimes.
There was a problem hiding this comment.
Isn't the problem that there is only one value? Otherwise, getLifetimeIteratorValue() would return an empty value.
There was a problem hiding this comment.
Ah yea, thats right, but it should be multiple values. I wonder why we are not getting both lifetime.
I assumed the error is that one variable has the lifetime v1 and the other has the lifetime v2. It should have both lifetimes.
| for (std::size_t i = 0; i < address1.size(); ++i) | ||
| for (std::size_t j = 0; j < address2.size(); ++j) | ||
| if (isSameExpression(true, false, address1[i], address2[j], library, false, false)) | ||
| return true; |
There was a problem hiding this comment.
I am not sure the reason for doing this, but it seems like it would be better to check if the entire set matches. This could be written as a nested std::any_of but it should probably be a nested std::all_of.
There was a problem hiding this comment.
It's for #12377. We used to get stuck on pv, which actually has two values (get1 or get2).
danmar
left a comment
danmar
left a comment
There was a problem hiding this comment.
lgtm.. feel free to merge if paul is happy..
|
@pfultz2 I think we should merge this, even if the ValueFlow is still wrong. As it stands, this check produces mostly FPs in daca. |
3 participants
…r to get reference to container